server/services/tuya/lib/tuya.poll.js (2)

102-154: ⚠️ Potential issue | 🟠 Major

Guard feature iteration against non-array payloads.

Line 126 calls deviceFeatures.forEach without verifying that deviceFeatures is an array. While line 104 checks for Array.isArray to set summary.polled, a non-array payload will still throw at line 126. This was flagged in a previous review.

🛠️ Proposed defensive fix

 const pollCloudFeatures = async function pollCloudFeatures(deviceFeatures, topic) {
   const summary = {
     polled: Array.isArray(deviceFeatures) ? deviceFeatures.length : 0,
     handled: 0,
     changed: 0,
     missing: 0,
     skipped: 0,
   };

+  if (!Array.isArray(deviceFeatures) || deviceFeatures.length === 0) {
+    return summary;
+  }
+
   if (!this.connector || typeof this.connector.request !== 'function') {
     logger.warn(`[Tuya][poll][cloud] connector unavailable for device=${topic}`);
     return summary;
   }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/services/tuya/lib/tuya.poll.js` around lines 102 - 154, The
pollCloudFeatures function currently calls deviceFeatures.forEach and can throw
if deviceFeatures is not an array; update pollCloudFeatures to guard the
iteration by either returning early when deviceFeatures is not an array
(consistent with how summary.polled is computed) or by iterating over
Array.isArray(deviceFeatures) ? deviceFeatures : [] so the loop never runs on
non-array payloads; reference the pollCloudFeatures function and the
deviceFeatures usage to locate where to add the guard.

---

225-255: ⚠️ Potential issue | 🟠 Major

Guard device.features iteration against non-array payloads.

device.features.forEach at line 225 assumes device.features is always an array. A malformed device object could throw and break polling for that device. This was flagged in a previous review alongside the pollCloudFeatures guard.

🛠️ Proposed defensive fix

+      const deviceFeatures = Array.isArray(device.features) ? device.features : [];
       const pendingCloudFeatures = [];

-      device.features.forEach((deviceFeature) => {
+      deviceFeatures.forEach((deviceFeature) => {
         const code = getFeatureCode(deviceFeature);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/services/tuya/lib/tuya.poll.js` around lines 225 - 255, The loop
assumes device.features is always an array and can throw for malformed devices;
before calling device.features.forEach in the polling routine, add a guard using
Array.isArray(device.features) (or coerce to an empty array) so the code
iterates only when features is an array; locate the snippet where
device.features.forEach is used (alongside helpers like getFeatureCode,
getLocalDpsFromCode, getFeatureReader, emitFeatureState, getCurrentFeatureState)
and either early-return for bad payloads or replace the forEach with iteration
over a safe const features = Array.isArray(device.features) ? device.features :
[].

server/services/tuya/lib/mappings/index.js (1)

115-131: ⚠️ Potential issue | 🟠 Major

Include specifications.properties in code extraction.

This function still only extracts codes from specifications.functions and specifications.status, missing codes that may appear in specifications.properties. This was flagged in a previous review and remains unaddressed.

🔧 Proposed fix

 const extractCodesFromSpecifications = (specifications) => {
   const codes = new Set();
   if (!specifications || typeof specifications !== 'object') {
     return codes;
   }
   const functions = Array.isArray(specifications.functions) ? specifications.functions : [];
   const status = Array.isArray(specifications.status) ? specifications.status : [];
+  const properties = Array.isArray(specifications.properties) ? specifications.properties : [];

-  [...functions, ...status].forEach((item) => {
+  [...functions, ...status, ...properties].forEach((item) => {
     if (!item || !item.code) {
       return;
     }
     codes.add(String(item.code).toLowerCase());
   });

   return codes;
 };

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/services/tuya/lib/mappings/index.js` around lines 115 - 131, The
extractCodesFromSpecifications function currently only reads
specifications.functions and specifications.status; update it to also read
specifications.properties (e.g., const properties =
Array.isArray(specifications.properties) ? specifications.properties : []) and
include those items when iterating so codes from properties are captured
(convert item.code to string, toLowerCase(), and add to the codes Set, same as
for functions/status) while keeping the existing null/undefined checks.

server/services/tuya/lib/mappings/index.js (1)

57-79: Verify that empty REQUIRED_CODES is intentionally permissive.

At line 73, hasRequiredCode is true when requiredCodes.size === 0, meaning device types without required codes will match based solely on modelName keywords. This may cause unintended matches if a future device type definition omits REQUIRED_CODES.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/services/tuya/lib/mappings/index.js` around lines 57 - 79, The
matchDeviceType function currently treats an empty REQUIRED_CODES set as
permissive (hasRequiredCode true), causing matches based solely on modelName;
change this so empty REQUIRED_CODES does not satisfy the requirement—update the
logic in matchDeviceType (use requiredCodes and hasRequiredCode) to require
requiredCodes.size > 0 and at least one code match (e.g., hasRequiredCode =
requiredCodes.size > 0 && Array.from(requiredCodes).some(code =>
codes.has(code))); alternatively, if permissive empty behavior is desired, make
it explicit by introducing a flag (e.g., ALLOW_EMPTY_REQUIRED_CODES) on the
typeDefinition and check that flag when computing hasRequiredCode.

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

server/services/tuya/lib/tuya.poll.js (1)

99-106: ⚠️ Potential issue | 🟠 Major

Guard feature iteration in both cloud and local paths.

Line 123 and Line 222 assume arrays and call .forEach directly. If device.features is malformed, polling throws and cloud fallback on Line 281 can fail for the same reason.

🛠️ Proposed defensive fix

 const pollCloudFeatures = async function pollCloudFeatures(deviceFeatures, topic) {
   const summary = {
     polled: Array.isArray(deviceFeatures) ? deviceFeatures.length : 0,
     handled: 0,
     changed: 0,
     missing: 0,
     skipped: 0,
   };
+
+  if (!Array.isArray(deviceFeatures) || deviceFeatures.length === 0) {
+    return summary;
+  }

   if (!this.connector || typeof this.connector.request !== 'function') {
     logger.warn(`[Tuya][poll][cloud] connector unavailable for device=${topic}`);
     return summary;
   }
@@
 async function poll(device) {
@@
   const params = device.params || [];
+  const deviceFeatures = Array.isArray(device.features) ? device.features : [];
@@
-        device.features.forEach((deviceFeature) => {
+        deviceFeatures.forEach((deviceFeature) => {
@@
-  cloudSummary = await pollCloudFeatures.call(this, device.features, topic);
+  cloudSummary = await pollCloudFeatures.call(this, deviceFeatures, topic);

Also applies to: 123-123, 222-223, 281-281

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/services/tuya/lib/tuya.poll.js` around lines 99 - 106, Guard array
iterations of device features in pollCloudFeatures and its local/cloud fallback
paths: check Array.isArray(device.features) (or deviceFeatures passed into
pollCloudFeatures) before calling .forEach or otherwise iterate; if not an
array, treat as empty (increment summary.missing/skipped as appropriate) or
return early. Update the loops referenced around the uses of device.features in
the cloud path and local fallback so they never call .forEach on a non-array and
ensure summary counters (polled/handled/changed/missing/skipped) are updated
consistently when features are malformed.

server/test/services/tuya/lib/tuya.localScan.test.js (2)

311-312: Add explicit assertion for ipParam existence.

If ipParam is undefined, accessing .value will throw a TypeError with a confusing message rather than a clear test failure. Adding an explicit existence check improves test diagnostics.

💡 Proposed fix

     const ipParam = response.devices[0].params.find((param) => param.name === 'IP_ADDRESS');
+    expect(ipParam).to.exist;
     expect(ipParam.value).to.equal('1.1.1.1');

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/test/services/tuya/lib/tuya.localScan.test.js` around lines 311 - 312,
Add an explicit assertion that the found parameter exists before accessing its
value: after computing ipParam (from response.devices[0].params.find(...)) add a
check like expect(ipParam).to.exist or expect(ipParam).to.be.ok to fail with a
clear message if the param is missing, then keep the existing
expect(ipParam.value).to.equal('1.1.1.1'); so the test reports a clear failure
when the IP param isn't present.

---

253-293: Test exercises code path but lacks assertions on logging behavior.

The test description states "should log listening address when available" but there's no assertion verifying that logging actually occurs. Consider adding a spy on the logger to verify the expected log message, or rename the test to clarify it's for coverage purposes.

💡 Proposed enhancement to verify logging

 it('should log listening address when available', async () => {
+  const loggerSpy = sinon.spy();
   const sockets = [];
   const dgramStub = {
     createSocket: () => {
       // ... existing socket setup ...
     },
   };

   // ... existing parser stub ...

   const { localScan } = proxyquire('../../../../services/tuya/lib/tuya.localScan', {
     dgram: dgramStub,
     '@demirdeniz/tuyapi-newgen/lib/message-parser': { MessageParser: MessageParserStub },
     '@demirdeniz/tuyapi-newgen/lib/config': { UDP_KEY: 'key' },
+    '../../../utils/logger': { debug: loggerSpy },
   });

   const clock = sinon.useFakeTimers();
   const promise = localScan(1);
   await clock.tickAsync(1100);
   await promise;
   clock.restore();
+
+  expect(loggerSpy.called).to.be.true;
 });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/test/services/tuya/lib/tuya.localScan.test.js` around lines 253 - 293,
The test exercises localScan but doesn't assert logging; update the test to
spy/stub the logger used by the module (e.g., the processLogger or logger
instance referenced inside localScan) before calling localScan, then assert that
the logger method (info/debug) was called with a message containing the
listening address and port '0.0.0.0:6666' (or a substring like '0.0.0.0' and
'6666'); alternatively, if you don't want to assert logging, rename the test to
reflect it's only exercising the code path for coverage. Ensure you reference
the localScan import from '../../../../services/tuya/lib/tuya.localScan' and
attach the spy before invoking localScan so the assertion can observe the log
call.

server/test/services/tuya/lib/tuya.poll.test.js (1)

341-581: Add one regression test for malformed features payloads.

Given the new branch-coverage block, a non-array features case would lock in graceful degradation and prevent runtime regressions.

🧪 Suggested test addition

 describe('TuyaHandler.poll additional branch coverage', () => {
+  it('should not throw when features payload is not an array', async () => {
+    const request = sinon.stub().resolves({ result: [{ code: 'switch_1', value: true }] });
+    const emit = sinon.stub();
+    const { poll } = proxyquire('../../../../services/tuya/lib/tuya.poll', {});
+
+    await poll.call(
+      {
+        connector: { request },
+        gladys: { event: { emit } },
+      },
+      {
+        external_id: 'tuya:device',
+        params: [{ name: 'LOCAL_OVERRIDE', value: false }],
+        features: null,
+      },
+    );
+
+    expect(emit.called).to.equal(false);
+  });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/test/services/tuya/lib/tuya.poll.test.js` around lines 341 - 581, Add
a regression test to the existing "TuyaHandler.poll additional branch coverage"
suite that calls the poll function (poll) with a device whose features property
is malformed (e.g., null or an object instead of an array) and asserts that poll
does not throw, logs a warning via the injected logger (logger.warn called),
does not emit events (gladys.event.emit not called), and does not crash the
connector.request; stub connector.request and gladys.event.emit as in the other
tests and place the test alongside the other it(...) blocks so malformed
features gracefully degrade.

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx (1)

488-488: ⚠️ Potential issue | 🟡 Minor

Use autoComplete casing in JSX (Line 488).

autocomplete should be autoComplete so the prop is correctly handled.

Suggested change

-                        autocomplete="off"
+                        autoComplete="off"

#!/bin/bash
# Read-only verification: find lowercase JSX autocomplete usage
rg -n -C2 '\bautocomplete=' --type js --type jsx

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx` at line 488,
In SetupTab.jsx update the JSX attribute name from the lowercase HTML form
attribute "autocomplete" to React/JSX camelCase "autoComplete" on the input
element inside the SetupTab component (the element currently rendering
autocomplete="off"); change the prop there to autoComplete="off" so React
recognizes and applies it correctly.

front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx (2)

29-34: Move side effects/subscriptions out of componentWillMount.

Using network calls and websocket listeners here is fragile with modern React/Preact lifecycle behavior; use componentDidMount for these side effects.

Suggested change

-  componentWillMount() {
+  componentDidMount() {
     this.getTuyaConfiguration();
     this.getTuyaStatus();
     this.props.session.dispatcher.addListener(WEBSOCKET_MESSAGE_TYPES.TUYA.STATUS, this.updateConnectionStatus);
     this.props.session.dispatcher.addListener(WEBSOCKET_MESSAGE_TYPES.TUYA.ERROR, this.displayConnectionError);
   }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx` around lines
29 - 34, The lifecycle side-effects (network calls and websocket subscriptions)
are placed in componentWillMount; move them to componentDidMount: call
this.getTuyaConfiguration() and this.getTuyaStatus() and register the listeners
via
this.props.session.dispatcher.addListener(WEBSOCKET_MESSAGE_TYPES.TUYA.STATUS,
this.updateConnectionStatus) and addListener(WEBSOCKET_MESSAGE_TYPES.TUYA.ERROR,
this.displayConnectionError) inside componentDidMount, and remove or leave
componentWillMount empty; ensure any corresponding cleanup (removing those
listeners) is done in componentWillUnmount.

---

492-500: Make the secret-toggle control keyboard-accessible.

A clickable <span> is not keyboard-friendly. Use a <button type="button"> with aria-label/aria-pressed.

Suggested change

-                    <span class="input-icon-addon cursor-pointer" onClick={this.toggleClientSecret}>
+                    <button
+                      type="button"
+                      class="input-icon-addon cursor-pointer"
+                      onClick={this.toggleClientSecret}
+                      aria-pressed={state.showClientSecret}
+                      aria-label="Toggle secret visibility"
+                    >
                       <i
                         class={cx('fe', {
                           'fe-eye': !state.showClientSecret,
                           'fe-eye-off': state.showClientSecret
                         })}
                       />
-                    </span>
+                    </button>

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx` around lines
492 - 500, The secret-toggle is a clickable <span> and must be
keyboard-accessible: replace the <span class="input-icon-addon cursor-pointer"
onClick={this.toggleClientSecret}> with a <button type="button"> that preserves
the same classes and onClick handler (toggleClientSecret) and add accessibility
attributes such as aria-label (e.g., "Toggle client secret visibility") and
aria-pressed={state.showClientSecret}; ensure the inner <i> still uses the
cx(...) logic for icon classes and the element remains focusable and styled like
the original control.

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx (1)

488-488: ⚠️ Potential issue | 🟡 Minor

Use autoComplete prop casing in JSX.

autocomplete at Line 488 should be autoComplete to match JSX prop conventions.

#!/bin/bash
# Verify JSX prop casing for autocomplete usage in this component.
rg -nP '\bautocomplete\s*=' front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@front/src/routes/integration/all/tuya/setup-page/SetupTab.jsx` at line 488,
In the SetupTab.jsx component change the JSX prop name from lowercase
"autocomplete" to the correct React/JSX casing "autoComplete" on the
input/element where it's currently set (search for the attribute in SetupTab JSX
markup); update any other occurrences found by the provided ripgrep command to
ensure consistent camelCase prop usage and run the component build/test to
verify no props are missed.

server/services/tuya/lib/tuya.loadDeviceDetails.js (1)

34-50: Consider extracting repeated error-logging logic.

The reason extraction pattern is duplicated four times. A small helper would reduce repetition.

♻️ Optional refactor

+const logIfRejected = (result, label) => {
+  if (result.status === 'rejected') {
+    const reason = result.reason?.message ?? result.reason;
+    logger.warn(`[Tuya] Failed to load ${label} for ${deviceId}: ${reason}`);
+  }
+};
+
+logIfRejected(specResult, 'specifications');
+logIfRejected(detailsResult, 'details');
+logIfRejected(propsResult, 'properties');
+logIfRejected(modelResult, 'thing model');
-  if (specResult.status === 'rejected') {
-    const reason = specResult.reason && specResult.reason.message ? specResult.reason.message : specResult.reason;
-    logger.warn(`[Tuya] Failed to load specifications for ${deviceId}: ${reason}`);
-  }
-  if (detailsResult.status === 'rejected') {
-    const reason =
-      detailsResult.reason && detailsResult.reason.message ? detailsResult.reason.message : detailsResult.reason;
-    logger.warn(`[Tuya] Failed to load details for ${deviceId}: ${reason}`);
-  }
-  if (propsResult.status === 'rejected') {
-    const reason = propsResult.reason && propsResult.reason.message ? propsResult.reason.message : propsResult.reason;
-    logger.warn(`[Tuya] Failed to load properties for ${deviceId}: ${reason}`);
-  }
-  if (modelResult.status === 'rejected') {
-    const reason = modelResult.reason && modelResult.reason.message ? modelResult.reason.message : modelResult.reason;
-    logger.warn(`[Tuya] Failed to load thing model for ${deviceId}: ${reason}`);
-  }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/services/tuya/lib/tuya.loadDeviceDetails.js` around lines 34 - 50,
Extract the repeated reason-extraction and logging into a small helper function
(e.g., getRejectionReason(result) and logRejection(result, label, deviceId)) and
replace the four duplicate blocks that use specResult, detailsResult,
propsResult, and modelResult with calls to that helper; getRejectionReason
should return result.reason.message if present or result.reason otherwise, and
logRejection should call logger.warn(`[Tuya] Failed to load ${label} for
${deviceId}: ${reason}`) only when result.status === 'rejected', keeping the
original labels ("specifications", "details", "properties", "thing model").

server/test/services/tuya/lib/utils/tuya.deviceParams.test.js (1)

30-35: Test descriptions are swapped relative to their content.

The test at line 30 claims "using device param constants" but actually uses plain string literals ('A', 'B'). Conversely, the test at line 51 has a generic description but actually uses DEVICE_PARAM_NAME constants. Consider swapping the descriptions to match the actual test content.

🔧 Proposed fix

-  it('should get param value using device param constants', () => {
+  it('should get param value', () => {
     const value = getParamValue([{ name: 'A', value: 42 }], 'A');
     expect(value).to.equal(42);
     expect(getParamValue([{ name: 'A', value: 42 }], 'B')).to.equal(undefined);
     expect(getParamValue(null, 'A')).to.equal(undefined);
   });

-  it('should get param value', () => {
+  it('should get param value using device param constants', () => {
     const params = [
       { name: DEVICE_PARAM_NAME.IP_ADDRESS, value: '1.1.1.1' },
       { name: DEVICE_PARAM_NAME.PROTOCOL_VERSION, value: '3.3' },
     ];

Also applies to: 51-58

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/test/services/tuya/lib/utils/tuya.deviceParams.test.js` around lines
30 - 35, The two unit tests around getParamValue have their descriptions
reversed: the test that uses plain string keys ('A', 'B') is labeled to use
device param constants and the test that actually uses DEVICE_PARAM_NAME
constants has a generic description; update the test descriptions so the one
invoking getParamValue([{ name: 'A', value: 42 }], 'A') reads something like
"should get param value using plain param names" and the one that passes
DEVICE_PARAM_NAME constants reads "should get param value using device param
constants" to match the actual test code (refer to the getParamValue calls and
DEVICE_PARAM_NAME usage to locate the tests).

server/services/tuya/lib/mappings/index.js (1)

137-141: Align external_id code parsing with the polling path.

This parser uses parts[2], while server/services/tuya/lib/tuya.poll.js (getFeatureCode) uses the last segment. Using a fixed index here is more brittle and can drift from polling behavior.

♻️ Proposed fix

-    const parts = String(feature.external_id).split(':');
-    if (parts.length >= 3) {
-      const code = parts[2];
+    const parts = String(feature.external_id).split(':');
+    if (parts.length >= 2) {
+      const code = parts[parts.length - 1];
       if (code) {
         codes.add(String(code).toLowerCase());
       }
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/services/tuya/lib/mappings/index.js` around lines 137 - 141, The code
in mappings/index.js extracts the feature code using parts[2], which can diverge
from the polling implementation getFeatureCode in
server/services/tuya/lib/tuya.poll.js that uses the last segment; modify the
parser in mappings/index.js (the block that splits feature.external_id and
currently reads parts[2]) to instead take the last segment (e.g.,
parts[parts.length - 1]) and normalize it the same way getFeatureCode does
(string coercion and toLowerCase) so both parsers remain consistent.

server/test/services/tuya/lib/tuya.localPoll.test.js (1)

16-146: Consider extracting a shared rejection assertion helper.

The repeated try/catch + throw new Error('Expected error') pattern appears across multiple tests. A small helper would reduce duplication and make future error-path additions less noisy.

Also applies to: 189-223

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/test/services/tuya/lib/tuya.localPoll.test.js` around lines 16 - 146,
Tests repeat the same try/catch + throw pattern to assert errors; create a small
helper (e.g., assertRejectsWith) that takes a function returning a promise, an
expected error class (BadParameters) and expected message or substring, and use
it to replace the repeated try/catch blocks in the tests that call localPoll
(the tests that currently define TuyAPIStub/TuyAPINewGenStub and then await
localPoll with invalid responses). Update each failing-path test (the ones
asserting instanceOf BadParameters and message checks) to call
assertRejectsWith(() => localPoll({...}), BadParameters, '...') instead of the
try/catch + throw to remove duplication while preserving the same assertions.

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro